Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow refreshing rates for unshipped complete orders #1906

Merged

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented May 13, 2017

When an order is complete, but not shipped yet, admins need to be able
to refresh rates for a shipment.

This is especially the case when splitting shipments to a new location.
This will create a new shipment with no rates, and refreshing them is the
only way to get any.

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 We probably want to have a changelog for this

@mamhoff mamhoff force-pushed the allow-refreshing-rates-for-unshipped-complete-orders branch from 72ae33a to 6ed9a78 Compare May 17, 2017 13:16
@mamhoff
Copy link
Contributor Author

mamhoff commented May 17, 2017

Added a changelog entry as well as a spec for the intended behaviour.

@jhawthorn
Copy link
Contributor

Just noting that previously refresh_rates was always called on completed orders 4ad32da. That might make this a less-scary change, but it might also make this change not achieve what is desired (correct updating of rates when the amount changes).

@mamhoff
Copy link
Contributor Author

mamhoff commented May 18, 2017

I part from the idea that it is possible to change the line items of an order when it is completed but not shipped yet. If that is possible, then the shipments must be able to change, too, as the line items will need to be shipped somehow.

@mamhoff mamhoff force-pushed the allow-refreshing-rates-for-unshipped-complete-orders branch from d06176f to 029aa49 Compare June 29, 2017 15:17
@mamhoff
Copy link
Contributor Author

mamhoff commented Jun 29, 2017

Rebased on current master.

@mamhoff
Copy link
Contributor Author

mamhoff commented Jul 1, 2017

@jhawthorn This is primarily intended to make splitting shipments for completed orders work. Right now it doesn't at all because the new shipment won't have rates.

@mamhoff mamhoff force-pushed the allow-refreshing-rates-for-unshipped-complete-orders branch from 029aa49 to 93fcf9a Compare July 7, 2017 13:27
@@ -72,12 +72,19 @@ def ship_shipment

expect(page).to have_css("#shipment_#{shipment1.id} tr.stock-item", count: 4)
shipment2 = (order.reload.shipments.to_a - [shipment1]).first
expect(page).to have_css("#shipment_#{shipment2.id} tr.stock-item", count: 1)
expect(page).to have_css("#shipment_#{shipment2.id} tr.stock-item", count: 1)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespace detected.

When an order is complete, but not shipped yet, admins need to be able
to refresh rates for a shipment.

This did not work for two reasons:
- `#refresh_rates` was not called on a new shipment
- If it had been called, it would not have done anything.

I added an expectation for a new shipment to be generated with a selected
shipping rate to test this behaviour.

This is a less scary change than it seems since the order updater does not
refresh rates automagically anymore.
@mamhoff mamhoff force-pushed the allow-refreshing-rates-for-unshipped-complete-orders branch from 93fcf9a to 9e145bd Compare July 7, 2017 13:28
@mamhoff mamhoff merged commit b8b2ccb into master Jul 10, 2017
@mamhoff mamhoff deleted the allow-refreshing-rates-for-unshipped-complete-orders branch July 10, 2017 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants